Skip to content

fix(aggregation): resolve signers from the head state's validator registry - #420

Merged
mananuf merged 4 commits into
fix/aggregation-skip-visibilityfrom
fix/aggregation-head-state-registry
Sep 1, 2026
Merged

fix(aggregation): resolve signers from the head state's validator registry#420
mananuf merged 4 commits into
fix/aggregation-skip-visibilityfrom
fix/aggregation-head-state-registry

Conversation

@mananuf

@mananuf mananuf commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Step 2 of #418: remove the failure mode that step 1 made visible.

Stacked on #419 — review that first; this PR's diff is against it.

The problem

aggregateFromSnapshot looked up a stored state per attestation target and dropped the group when that state was absent:

targetState := snap.targetStates[attData.Target.Root]
if targetState == nil {
    return
}

On devnet-5 (2026-08-31, 14:35–15:04 UTC) that was every group, every slot. gean_7 produced nothing in 355 of 356 slots while healthy (Behind: 0–3) and holding 753 signatures, each session finishing in 14–90 µs. It held 262 states against a head of slot 399, and the votes it was aggregating carried target slot 342.

Why the lookup was never needed

Every use of that state was state.Validators[...] — resolving attestation pubkeys, nothing else.

The validator registry is written once, in generate_genesis, and never by the state transition. I checked the pinned spec (lstar/state_transition.py): validators= appears only in genesis construction, and gean's internal/statetransition never assigns to .Validators either.

So every state on the chain carries the same registry, and the head state resolves exactly the same signers. SnapshotInputs already requires a head state and returns nil without one, so it is always available — unlike an arbitrary target's state.

What changes

  • Signers resolve against snap.headState.Validators.
  • The per-target state map and the store read per data root that filled it are gone.
  • The missing_target_state skip reason is removed: the condition it named can no longer occur.

Selection, ordering, budget and the resulting aggregates are otherwise unchanged.

How ethlambda does it

crates/blockchain/src/aggregation.rs builds one ProjectedState from the head state and validates candidates against that chain view. It has never needed a per-target stored state; this brings gean to the same footing.

Risk

The change rests on the registry being genesis-fixed. If a future spec adds validator set changes, resolving against the head would be wrong for a target under a different registry — the reasoning is recorded in a comment at the call site so it surfaces during that work.

Testing

  • make test — all 25 packages green; make lint, go vet clean; -race clean on aggregation, node, store.
  • New test asserts a group whose target has no stored state now reaches signer selection instead of being dropped.
  • Two existing fixtures needed correcting, and the reason is worth noting: aggregateTestSnapshot built a snapshot with no head state and votes targeting slot 0 — neither of which SnapshotInputs can produce. They now carry a head state and a target slot matching the vote slot.

…istry

A session looked up a stored state per attestation target and dropped
the group when that state was absent. On devnet-5 that was every group,
every slot: an aggregator holding 753 signatures produced nothing for
355 consecutive slots while its head tracked the chain.

The lookup was never necessary. The registry is written once in
generate_genesis and never by the state transition, so every state on
the chain carries the same validators and the head state — which
SnapshotInputs already requires — resolves the same signers. Using it
removes the failure mode rather than reporting it.

This also drops a store read per data root from the snapshot and the
per-target state map it filled.

The missing_target_state skip reason goes with it; the condition it
named can no longer occur.

Refs #418
The lean_attestation_aggregate_coverage_* gauges were registered but
never set, so gean published no series for them and could not appear on
the shared devnet Aggregation Coverage panels at all. That is half the
reason an aggregator producing nothing went unnoticed: the panel an
operator checks first is blank for gean whether or not it is working.

Emits the same sections and labels the other clients use, so the numbers
line up on one panel:

  timely      new payloads captured before the tick promotes them
  late        new payloads for the same round arriving after
  block       votes for the round carried by the canonical head block
  combined    union of the three
  agg_start_new  what the session is about to work from, at interval 2
  proposal_combined  what our own proposal covers

plus the block-vs-timely symmetric difference. Subnet split is
validator_id % committee_count, matching p2p.SubnetID.

The four post-block sections record a genuine all-zero reading: an empty
slot is real information and should read as a dip, not as a gauge
holding its last value. The diff gauges instead keep their previous
value until a block has reported the round, since before that the
comparison is undefined rather than empty.

Pure observability; nothing here feeds fork choice or the transition.

Refs #418
@mananuf
mananuf changed the base branch from fix/aggregation-skip-visibility to main September 1, 2026 16:25
@mananuf
mananuf changed the base branch from main to fix/aggregation-skip-visibility September 1, 2026 16:26
mananuf and others added 2 commits September 1, 2026 17:50
A single-aggregator devnet leaves the payload buffers empty whenever the
emitters read them, so the sections there are legitimately zero and prove
nothing. These feed known votes for a round straight into the reporter
and assert the block/timely split and the symmetric difference, plus the
empty-round and no-head-state paths.

Refs #418
feat(metrics): report attestation-aggregate coverage
@mananuf
mananuf merged commit e5c96ce into fix/aggregation-skip-visibility Sep 1, 2026
1 check passed
@mananuf
mananuf deleted the fix/aggregation-head-state-registry branch September 1, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant